Conversation
UiHyeon-Kim
left a comment
There was a problem hiding this comment.
3주차 고생하셨습니다~
전반적인 코드는 잘 작성하신 것 같아요.
만약 MVC 패턴을 적용하고 싶으시다면, 따로 View 패키지를 만들어 입출력만 담당하게 하면 좋을 것 같아요. repository는 CRUD (즉, Model)를 담당하고, service는 비즈니스 로직을 담당을 하고 있습니다. 여기서 출력 부분만 따로 빼주신다고 생각하면 되는데, 이 또한 내부에 객체를 생성하는 것보다 생성자 주입하는 것이 OOP 를 지키기 위한 길이라고 생각이 듭니다.
|
|
||
| public class CustomerService { | ||
| ArrayList<CustomerLegacy> customers = new ArrayList<>(); | ||
| UserService userService = new UserServiceImpl(); |
There was a problem hiding this comment.
UserService를 인터페이스로 추상화 한 것은 매우 좋다고 생각합니다.
그런데 인터페이스의 구현체를 CustomerService 내부에 객체로 생성하면 인터페이스로 추상화 한 이점이 살지 않습니다.
또한 비즈니스 로직같은 상위 모듈은 데이터, 모델 같은 하위 모듈을 직접 참조하면 안됩니다.
객체지향 원칙에는 DIP 라는 것이 있는데 한 번 찾아보시면 도움이 되실 것 같아요.
| import java.util.ArrayList; | ||
|
|
||
| public class UserServiceImpl implements UserService { | ||
| UserRepository userRepository = new UserRepositoryImpl(); |
| ArrayList<User> users = userRepository.getUsers(); | ||
| System.out.println("----- 고객 조회 결과 -----"); | ||
| System.out.println("총 고객 수: " + users.size()); | ||
| int i = 0; |
There was a problem hiding this comment.
대부분의 변수명은 괜찮았는데 이부분은 조금 아쉽네요.
제가 문제를 알고 있어서 금방 이해할 수 있었지만, 몰랐더라면 조금 아쉬운 코드가 될 것 같아요.
그리고 번호가 1번부터 시작해야 하는 것으로 아는데 i++ 로 진행하면 0부터 나올 것 같은데 한 번 확인해 보시면 좋을 것 같아요.
| } | ||
| else{ | ||
| status = "비활성"; | ||
| } |
There was a problem hiding this comment.
자바를 사용한다면 이런 간단한 조건은 3항 연산을 사용하면 더 깔끔하게 보일 것 같아요!
그리고 toString을 활용한 것이 굉장히 좋아 보입니다!
시간이 없어서 추가 기능은 구현 못했습니다...
MVC 구조를 처음 해봐서 흉내만 내봤는데 개선점이 무엇인지 궁금합니다